-
Notifications
You must be signed in to change notification settings - Fork 287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: adds a couple of new commands for GMs on games. #6183
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6183 +/- ##
============================================
- Coverage 29.17% 29.15% -0.03%
+ Complexity 13971 13969 -2
============================================
Files 2628 2636 +8
Lines 266564 266772 +208
Branches 47575 47594 +19
============================================
- Hits 77771 77770 -1
- Misses 184910 185119 +209
Partials 3883 3883 ☔ View full report in Codecov by Sentry. |
@@ -0,0 +1,131 @@ | |||
/* | |||
* Copyright (C) 2024 Luana Scoppio ([email protected]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you claiming copyright of this code? Normally we ask that the code be under the MegaMek team copyright, I believe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just confirmed with the team, this isn’t the issue I thought it was, so carry on! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kinda copied and pasted what were in other files, since this is based directly in an older file that is the same license I tought that adding my name to it to make clear who authored would be reasonable.
But either way, I will review if I didnt forget the megamek team entry in any of the files.
@@ -140,6 +140,8 @@ GameOptionsInfo.option.allow_nukes.displayableName=Allow command-line nukes. | |||
GameOptionsInfo.option.allow_nukes.description=This must be checked to allow players to throw nukes from the command line. If this is not checked, nukes are still available as advanced munitions. | |||
GameOptionsInfo.option.really_allow_nukes.displayableName=REALLY allow command-line nukes. | |||
GameOptionsInfo.option.really_allow_nukes.description=This must be checked to allow players to throw nukes from the command line. If this is not checked, nukes are still available as advanced munitions. | |||
GameOptionsInfo.option.gm_can_kill_units.displayableName=Allow command-line kill (GM only). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest lumping this together with command line nukes. I doubt the distinction is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe one new specifically to enable/disable GM commands in game? Because now I have created 4 new GM-only commands, or maybe just not add the option at all, since you are turning yourself into a GM, requesting the blessing of the other players, maybe having access to all commands should be a given instead of having multiple flags blocking you from doing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I would go with "GM gets access to all GM commands by default". Nominally, the point is to give them access to various tools that let them arbitrarily alter the scenario as needed. The GM status itself already has some safeguards applied to prevent abuse (can't just activate it midgame without the players voting to allow it, etc.).
…nditions, orbital bombardment, etc.
|
||
private void updatePlanetaryCondition(String arg, String prefix, int connId, int maxLength, Consumer<Integer> setter, | ||
Function<Integer, String> successMessage, Function<Integer, String> errorMessage) { | ||
var value = Integer.parseInt(arg.substring(prefix.length())); |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note
} | ||
Report.addNewline(vDesc); | ||
} | ||
} | ||
} | ||
|
||
private void explosionDamage(Coords position, Vector<Report> vDesc, Entity e, int damage) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
|
||
// Next, for whatever's left, do terrain effects | ||
// such as clearing, roughing, and boiling off water. | ||
boolean damageFlag = true; |
Check notice
Code scanning / CodeQL
Unread local variable Note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw you requested I review this.
I don’t have enough working knowledge of the mm side of our codebase, so try not to review mm-specific stuff unless it’s super basic - and even then, I prefer not to get involved. Didn’t want you thinking I was just ignoring your request. :)
New commands:
/kill : Kills (ID) unit at GM's discretion, needs to be enabled in the game options.
/nosmoke: clears all smoke clouds in the board.
/weather [...args]: allows the GM to change the weather on demand
/disaster [type]: cast a disaster (like in the old days of SimCity) to spicy things up (WIP)!